fix: skip permissions missing from destination during role sync#503
Closed
nathantournant wants to merge 3 commits intomainfrom
Closed
fix: skip permissions missing from destination during role sync#503nathantournant wants to merge 3 commits intomainfrom
nathantournant wants to merge 3 commits intomainfrom
Conversation
…_update_sync In test_resource_update_sync, the --filter argument was appended to diff_cmd instead of sync_cmd due to a copy-paste error. This caused the sync step to run unfiltered in live integration tests (RECORD=none), potentially syncing resources outside the test scope. Cassette-based tests masked the issue since VCR replays fixed responses regardless. Adds regression tests validating that command-building logic places filter arguments in the correct command lists. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
5869c12 to
417bf57
Compare
a6a9f99 to
b9ea16b
Compare
8cc4b00 to
39c74b5
Compare
When syncing roles cross-DC, permissions gated behind feature flags (e.g. bits_dev_write) may exist in the source org but not the destination. Previously, remap_permissions left the unresolved permission name as a raw string in the payload, causing the Roles API to reject it with "invalid UUID [permission_name]". Now, permissions that exist in the source but have no match in the destination are dropped from the role payload with a warning log. This prevents a single missing permission from breaking the entire role sync — and by extension every integration test teardown. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
39c74b5 to
3a878c7
Compare
The Notebooks API now injects ai_generated, ai_edited, and human_edited tags on every write based on request characteristics. Since the sync CLI uses API key auth (not MCP), every PUT is classified as a human edit, causing human_edited:true to be added server-side. This creates a non-converging diff because the source notebook lacks the tag while the destination always gets it re-injected. Strip these server-managed tags in handle_special_case_attr so they are ignored on both sides during import, sync, and diff. Legitimate user tags (team:*, llm-observability:*) are unaffected. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
8037480 to
a6b4c19
Compare
Member
Author
|
Changes ported to #502 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two fixes for cross-DC integration test stability:
remap_permissionsnow drops permissions that exist in the source but not the destination (e.g.bits_dev_writebehind a feature flag), instead of leaving raw name strings that cause400 Bad Request: invalid UUIDattributes.tagstoexcluded_attributes— the Notebooks API injectshuman_edited:trueon update, causing phantom diffs that never convergeRoot cause
beta-permission-bits_dev_writefeature flag enabled; the destination org (US5) does not. Standard roles include this permission, and cross-DC sync failed on every role operation.human_edited:true, a server-managed tag not present in source resource files.Test plan
🤖 Generated with Claude Code